fix: Add integrations tests for browser and node#4390
Merged
GordonSmith merged 1 commit intoJun 20, 2025
Conversation
jeclrsg
approved these changes
Jun 12, 2025
Contributor
jeclrsg
left a comment
jeclrsg
left a comment
There was a problem hiding this comment.
A good chunk (maybe all) of that content in this instructions.md seems like it'd be useful to have under the "Developer Zone" heading of the repo's readme?
GordonSmith
force-pushed
the
COPILOT_INSTRUCTIONS
branch
from
416e67d to
859ddc9
Compare
Member
Author
|
@jeclrsg - I let copilot redo the main readme and added some "integration" tests that checks the NodeJS CJS / ESM + Browser UMD / ESM loading... |
GordonSmith
force-pushed
the
COPILOT_INSTRUCTIONS
branch
from
859ddc9 to
77364da
Compare
jeclrsg
requested changes
Jun 13, 2025
Contributor
jeclrsg
left a comment
jeclrsg
left a comment
There was a problem hiding this comment.
@GordonSmith noticed a few formatting issues in the tests. also, the GH actions was noting some issues with a couple of timeouts on the beforeEach that were importing things
GordonSmith
force-pushed
the
COPILOT_INSTRUCTIONS
branch
4 times, most recently
from
361e6e9 to
77c4a9e
Compare
GordonSmith
changed the title
GordonSmith
force-pushed
the
COPILOT_INSTRUCTIONS
branch
3 times, most recently
from
09661de to
764f596
Compare
GordonSmith
commented
Jun 18, 2025
GordonSmith
force-pushed
the
COPILOT_INSTRUCTIONS
branch
2 times, most recently
from
b00fdf5 to
dfdb6c3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds integration tests for browser and Node.js environments for the @hpcc-js packages while also updating configuration files, documentation, and workflows to support multiple module formats. Key changes include:
- Addition of browser UMD and ESM test suites (including updated Vitest configs and tsconfig files) for the @hpcc-js/util, @hpcc-js/dataflow, and @hpcc-js/comms packages.
- Updates to package.json scripts and dependency declarations (including adjustments in the comms package) to support expanded testing.
- Enhancements to documentation (README files and GitHub workflow configuration) to clarify usage and integration testing.
Reviewed Changes
Copilot reviewed 20 out of 37 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/browser-umd/.ts, tests/browser-esm/.ts | New test suites for both UMD and ESM browser environments |
| packages/markdown-it-plugins/tests/* | Minor path update in fetch() call |
| packages/comms/package.json | Dependency declarations updated (moved some deps into devDependencies) |
| package.json | Updated scripts (including uninstall command) and workspaces |
| README.md (root and tests/) | Extensive documentation updates for setup, testing, and usage |
| GitHub workflows (.github/workflows/release-please.yml) | Added extra test commands for browser and Node.js ESM/CJS tests |
| .github/instructions/general.instructions.md | New internal instructions for project standards and practices |
Comments suppressed due to low confidence (2)
packages/comms/package.json:89
- The dependencies d3-array, d3-format, d3-time-format, data-uri-to-buffer, safe-buffer, and tmp have been moved from dependencies to devDependencies. Please confirm that these modules are not required at runtime in production, as this change could lead to missing modules when the package is used.
"d3-array": "^1",
package.json:15
- The uninstall script now removes package-lock files and node_modules from several locations. Please verify that these removals are intentional and do not delete lock files that may be needed for proper dependency management in your environments.
"uninstall": "lerna clean && rimraf --glob packages/**/node_modules packages/**/package-lock.json demos/**/node_modules demos/**/package-lock.json tests/**/node_modules package-lock.json tests/**/package-lock.json node_modules",
jeclrsg
requested changes
Jun 19, 2025
Contributor
jeclrsg
left a comment
jeclrsg
left a comment
There was a problem hiding this comment.
@GordonSmith noticed a few more nitpick formatting issues. And I don't understand those tests about "different types of exports"?
GordonSmith
force-pushed
the
COPILOT_INSTRUCTIONS
branch
from
dfdb6c3 to
af4ceac
Compare
jeclrsg
requested changes
Jun 19, 2025
Contributor
jeclrsg
left a comment
jeclrsg
left a comment
There was a problem hiding this comment.
@GordonSmith just one last nitpick where those test instances are missing whitespace between them
| expect(connection).toBeDefined(); | ||
| expect(wuService).toBeDefined(); | ||
| expect(dfuService).toBeDefined(); | ||
| }); it("should work in browser environment", async () => { |
Contributor
There was a problem hiding this comment.
guess I missed it earlier, but last one of these I think?
Resolve minor export issues Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
GordonSmith
force-pushed
the
COPILOT_INSTRUCTIONS
branch
from
af4ceac to
f09add9
Compare
jeclrsg
approved these changes
Jun 19, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist:
Testing: